Specialize mapreduce + sparsevec on function arguments - #753
Conversation
`Base._mapreduce`, `_mapreducezeros` and `Base._mapreducedim!` for sparse matrices, their sparse-vector counterparts, and the `sparsevec` methods with a `combine` argument took their function arguments without a type parameter. Where such an argument is only passed through, Julia compiles the method for `Function` rather than for the concrete function, and every call below that point becomes dynamic. Under juliac's `--trim` this made `sum(A)` for a sparse matrix, and the other reductions with it, unbuildable. Give each of these function arguments a type parameter, as JuliaSparse#751 did for the `combine` argument of `sparse`, so the methods specialize and the reduction call graph resolves statically. This commit was written with the assistance of generative AI (Claude). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #753 +/- ##
==========================================
- Coverage 83.85% 83.84% -0.01%
==========================================
Files 13 13
Lines 9387 9386 -1
==========================================
- Hits 7871 7870 -1
Misses 1516 1516 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
You could write a comment explaining why the extra type parameters are needed - they look otherwise useless and somebody else might remove them. |
fredrikekre
left a comment
fredrikekre
left a comment
There was a problem hiding this comment.
You could write a comment explaining why the extra type parameters are needed
I don't think that is needed. This is a well known pattern and is used all over the place (https://docs.julialang.org/en/v1/manual/performance-tips/#Be-aware-of-when-Julia-avoids-specializing).
|
I agree with @fredrikekre but I am happy to add a note if you'd like @araujoms |
3 participants
Another small trim fix-up, follow-up to #751 . Expect ~1 or 2 more PRs in this series.
I do not think it is easy to add a test here since the problem this fixes has to do with the "compileable" specializations chosen by the compiler - those affect codegen and cause
--trimto fail but they don't generally impact inference.